Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): fix ng-prop-* with undefined values - #16798

Merged
jbedard merged 1 commit into
angular:masterfrom
jbedard:16797
Jan 11, 2019
Merged

fix($compile): fix ng-prop-* with undefined values#16798
jbedard merged 1 commit into
angular:masterfrom
jbedard:16797

Conversation

@jbedard

@jbedard jbedard commented Dec 22, 2018

Copy link
Copy Markdown
Contributor

Fixes #16797

Comment thread src/ng/compile.js Outdated
Comment thread test/ng/ngPropSpec.js Outdated
Comment thread test/ng/ngPropSpec.js
var element = $compile('<span ng-prop-text="myText" />')($rootScope);
// Initialze to non-falsey
$rootScope.myText = 'abc';
$rootScope.$digest();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I like to say, you can make these shorter with: $rootScope.$apply('text = ...') 😁

Comment thread src/ng/compile.js Outdated

@gkalpak gkalpak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Might be worth adding a test to ensure that values do not go through .prop() or that special property names (such as class) are not handled specially.

@jbedard

jbedard commented Dec 30, 2018

Copy link
Copy Markdown
Contributor Author

I've added 2 tests: 8b9cb92

Comment thread src/ng/compile.js
function applyPropValue() {
var propValue = ngPropGetter(scope);
$element.prop(propName, sanitizer(propValue));
$element[0][propName] = sanitizer(propValue);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$element can only ever have 1 element at this point, right? 😟

@jbedard
jbedard merged commit 8b973e0 into angular:master Jan 11, 2019
jbedard added a commit that referenced this pull request Jan 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants